Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not compile regexes on each run #3949

Merged
merged 2 commits into from
Jun 20, 2019
Merged

Conversation

albertvaka
Copy link
Contributor

  • Keep a cache of already compiled regexes (they will always be the same)
  • Alphanumeric strings an be simply matched for == instead of using RE.

- Keep a cache of already compiled regexes (they will always be the same)
- Alphanumeric strings an be simply matched for `==` instead of using RE.
truthbk
truthbk previously approved these changes Jun 20, 2019
Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, particularly interesting regarding your insights into how re caches the regexes.

except Exception:
self.warning("Cannot compile regex: %s" % regex)
return []
if key.isalnum():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the isalnum trick makes a big performance difference, if not a memory one. Some minor comments though.

go_expvar/datadog_checks/go_expvar/go_expvar.py Outdated Show resolved Hide resolved
go_expvar/datadog_checks/go_expvar/go_expvar.py Outdated Show resolved Hide resolved
@albertvaka albertvaka force-pushed the albertvaka/reduce-re-compile branch from 309ab83 to 407d786 Compare June 20, 2019 13:58
@codecov
Copy link

codecov bot commented Jun 20, 2019

Codecov Report

Merging #3949 into master will increase coverage by 9.41%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##           master    #3949      +/-   ##
==========================================
+ Coverage   79.47%   88.88%   +9.41%     
==========================================
  Files         158        7     -151     
  Lines        8250      270    -7980     
  Branches     1007       43     -964     
==========================================
- Hits         6557      240    -6317     
+ Misses       1460       19    -1441     
+ Partials      233       11     -222

@albertvaka albertvaka merged commit 2b3025b into master Jun 20, 2019
@albertvaka albertvaka deleted the albertvaka/reduce-re-compile branch June 20, 2019 14:36
ofek pushed a commit that referenced this pull request Jun 20, 2019
* Do not compile regexes on each run

- Keep a cache of already compiled regexes (they will always be the same)
- Alphanumeric strings an be simply matched for `==` instead of using RE.

* Deduplicate loop as per CR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants